feat(skills): add rhdh-must-gather-helm-bump skill and script - #86
feat(skills): add rhdh-must-gather-helm-bump skill and script#86nickboldt wants to merge 12 commits into
Conversation
Automate Helm CLI version bumps across rhdh-must-gather upstream and rhidp/rhdh distgit, including Konflux .tekton prefetch (CGW generic vs vendored gomod). Covers the multi-repo workflow introduced for RHIDP-16046. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Restructure SKILL.md with verification gates, success criteria, and E2E triage pointers from rhdh-must-gather#284. Add references for Helm 4 OCI stdout, Konflux/E2E verification, and CGW flat tarball layout. Script now updates sync/upstream_SHA_rhdh-must-gather and removes stale distgit vendor/helm on the CGW path. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Reflect merged upstream checksum verify (verify-helm-tarball.sh), TARGETPLATFORM arch field-2 notes, and midstream !697 lesson: sync hack/ into distgit and stop excluding it in upstream_repos.yml so Stage 2a COPY does not break after bot sync. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Midstream main already keeps hack/ in sync; the bump skill only needs to copy verify-helm-tarball.sh and document #284 hardening (checksum verify, TARGETPLATFORM field-2, E2E pipefail). Remove ensure_hack_not_excluded and related docs/tests. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Scope components.yaml prefetch to must-gather only, stop copying upstream root Containerfile onto hermetic distgit, regenerate distgit Containerfile from .rhdh/docker (preserving VERSION/MIDSTREAM_REPO), flip Stage 2a/2b bidirectionally, sync whole hack/ and vendor/ (omit helm on CGW), and tighten script guards plus skill prose. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve README conflict with main's catalog-first layout, move rhdh-must-gather-helm-bump under skills/ci/, register it in catalog.json / marketplace / ask-rhdh routes, and satisfy catalog Completion + name-only composition rules. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
CI lint job failed on `ruff format --check` for the new unit test file. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Regenerating from .rhdh/docker/Containerfile was dropping the Brew/Konflux ENV SUMMARY/LABEL block that sync-midstream appends. Keep that footer, and stop flipping upstream Containerfiles on --skip-upstream. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
When regenerating distgit Containerfile from .rhdh/docker, increment LABEL release and the matching konflux.additional-tags version-release suffix (e.g. 2.0-1 → 2.0-2), mirroring sync-midstream.sh. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Match the rhdh-skill convention of Python bundled scripts so the Helm bump workflow is easier to test and maintain than the previous bash implementation, while preserving CGW/vendor sync and Tekton prefetch behavior. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
schultzp2020
left a comment
There was a problem hiding this comment.
The CGW vs vendor split, and editing only must-gather's prefetch, are the right idea. I'd tighten the apply path before this runs against rhidp/rhdh. --check and --skip-upstream can pick different modes, 2.0-10 becomes 2.0-20, and a failed git status looks clean. Tests don't hit those cases. On Windows the new file fails on shebang exec (WinError 193) before rsync is involved.
Comments below are the hunks I'd change. While you're there: don't write upstream_SHA from a dirty HEAD; require hack/check-helm-binary-available.sh and turn a missing bash/rsync into die(); run tests via sys.executable and actually bump --to off the 4.2.3 fixture (the lockfile stubs are no-ops); cover --dry-run, push.yaml, and 2b back to 2a. Same skills/ci/... path is in the script docstring. PR body still names bump-must-gather-helm.sh.
| ## Usage | ||
|
|
||
| ```bash | ||
| SKILL=skills/ci/rhdh-must-gather-helm-bump # under rhdh-skill checkout |
There was a problem hiding this comment.
The category folder is gone after install, so an agent that copies this Usage block misses the script and starts editing Tekton by hand. rhdh-yarn-bump uses a placeholder for that reason.
| SKILL=skills/ci/rhdh-must-gather-helm-bump # under rhdh-skill checkout | |
| SKILL=<this skill's directory> |
| 4. Run `--dry-run`, then the script without `--dry-run`. | ||
| 5. Review `git diff` in **both** repos. | ||
| 6. Run every verification gate — read [references/verification.md](references/verification.md). | ||
| 7. Commit / PR·MR only when the user asks. | ||
|
|
There was a problem hiding this comment.
--check and --dry-run can stay ungated. The next line writes two other working trees. /mutation-gate calls that a write. rhdh-base-images gates the apply; rhdh-prow-trigger says a dry-run is not approval to execute. Completion below currently treats the real run as done before anyone signed off.
| 4. Run `--dry-run`, then the script without `--dry-run`. | |
| 5. Review `git diff` in **both** repos. | |
| 6. Run every verification gate — read [references/verification.md](references/verification.md). | |
| 7. Commit / PR·MR only when the user asks. | |
| 3. Run `--check` and `--dry-run`. That pair is the preview. | |
| 4. Apply only after `/mutation-gate` approval, with upstream and distgit/Tekton as separate operations. Approval to dry-run is not approval to write. | |
| 5. Review `git diff` in both repos, then the gates in [references/verification.md](references/verification.md). | |
| 6. Commit / PR or MR only when the user asks, again through `/mutation-gate`. |
| check_only: bool, | ||
| ) -> str: | ||
| mode = "cgw" | ||
| if upstream is not None and (not skip_upstream or check_only): |
There was a problem hiding this comment.
If upstream is the sync source, probe it on apply the same way you do for --check. --skip-upstream currently looks at distgit: leftover vendor/helm (the test fixture always has this) plus an old lock for a different version becomes mode=vendor, while --check still prints mode=cgw. Then you copy a CGW lockfile and set gomod prefetch.
| if upstream is not None and (not skip_upstream or check_only): | |
| if upstream is not None: |
| result = subprocess.run( | ||
| ["git", "-C", str(path), "status", "--porcelain"], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if result.stdout.strip(): |
There was a problem hiding this comment.
check=False plus "empty stdout means clean" treats fatal: not a git repository as success. That path is rc 128 with empty stdout, then distgit still gets written. Tests always pass --allow-dirty, so this never runs.
| result = subprocess.run( | |
| ["git", "-C", str(path), "status", "--porcelain"], | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| ) | |
| if result.stdout.strip(): | |
| result = subprocess.run( | |
| ["git", "-C", str(path), "status", "--porcelain"], | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| ) | |
| if result.returncode != 0: | |
| die( | |
| f"{label} is not a git repository or git status failed ({path}): " | |
| f"{result.stderr.strip() or 'unknown error'}" | |
| ) | |
| if result.stdout.strip(): |
| if version: | ||
| footer = footer.replace(f"{version}-{current}", f"{version}-{next_n}") | ||
| else: | ||
| footer = re.sub( | ||
| rf'(konflux\.additional-tags="[^\"]*-){re.escape(current)}', | ||
| rf"\g<1>{next_n}", | ||
| footer, | ||
| ) |
There was a problem hiding this comment.
replace("2.0-1", "2.0-2") also rewrites 2.0-10. Same hole in the no-version regex. Add a test with release="10" / 2.0-10; the fixture only covers 1 -> 2.
| if version: | |
| footer = footer.replace(f"{version}-{current}", f"{version}-{next_n}") | |
| else: | |
| footer = re.sub( | |
| rf'(konflux\.additional-tags="[^\"]*-){re.escape(current)}', | |
| rf"\g<1>{next_n}", | |
| footer, | |
| ) | |
| footer = footer.replace(f'release="{current}"', f'release="{next_n}"') | |
| if version: | |
| footer = re.sub( | |
| rf"{re.escape(version)}-{re.escape(current)}(?![0-9])", | |
| f"{version}-{next_n}", | |
| footer, | |
| ) | |
| else: | |
| footer = re.sub( | |
| rf'(konflux\.additional-tags="[^\"]*-){re.escape(current)}(?![0-9])', | |
| rf"\g<1>{next_n}", | |
| footer, | |
| ) |
| die(f"Could not find must-gather under {parent}") | ||
| upstream = found | ||
| if downstream is None: | ||
| found = discover_repo(parent, "4-rhdh", "rhdh", "rhidp-rhdh") |
There was a problem hiding this comment.
--parent-dir never looks for rhdh-downstream, which is the name /rhdh-context uses. rhdh is the GitHub clone. Validate fails (no silent write), but a sibling GitLab checkout is never tried. Stronger still: only accept a candidate that has distgit/containers/rhdh-must-gather.
| found = discover_repo(parent, "4-rhdh", "rhdh", "rhidp-rhdh") | |
| found = discover_repo(parent, "4-rhdh", "rhdh-downstream", "rhidp-rhdh") |
Address review on the apply path: probe upstream for CGW vs vendor on apply, fail git status errors, numeric-boundary release tags, discover rhdh-downstream, gate writes behind /mutation-gate, and run tests via sys.executable with a real --to bump. https://redhat.atlassian.net/browse/RHIDP-16046 Generated-by: cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
Adds the
rhdh-must-gather-helm-bumpskill (skills/ci/) and bundledscripts/bump-must-gather-helm.pyto propagate a Helm CLI version bump across:redhat-developer/rhdh-must-gatherMakefileHELM_VERSION, CGWartifacts.lock.yamlorvendor/helm/, Stage 2a/2b onContainerfileand.rhdh/docker/Containerfilegitlab.cee.redhat.com/rhidp/rhdhdistgit/containers/rhdh-must-gather/; regenerate the hermetic distgitContainerfile; patch must-gather Tekton prefetch; writesync/upstream_SHA_rhdh-must-gatherThis is the Helm CLI in the must-gather image, not the RHDH Helm chart (
oci://quay.io/rhdh/chart). The script does not commit, push, or open a PR/MR.Catalog: registered under
skills/ci/incatalog.json, marketplace, and/ask-rhdhroutes.https://redhat.atlassian.net/browse/RHIDP-16046
What the script does
Preview (ungated):
--checkprobes CGW and printshelm_version=/mode=cgw|vendor.--dry-runprints the planned writes without touching trees. Approval to preview is not approval to apply.Apply (after
/mutation-gate; upstream and distgit/Tekton as separate operations):hack/check-helm-binary-available.sh(same probe for--checkand apply, including--skip-upstream):hack/update-helm-lockfile.sh→ Konfluxgenericprefetch on distgit root → Stage 2a activehack/update-vendor.sh helm→gomodprefetch onvendor/helm→ Stage 2b activeContainerfile,.rhdh/docker/Containerfile, and the regenerated distgitContainerfile.Makefile,artifacts.lock.yaml, entirehack/,.rhdh/docker/Containerfile, entirevendor/(on CGW, omit/deletevendor/helm; keepwebsocatetc.). Does not copy upstream rootContainerfileonto hermetic distgit.Containerfilefrom.rhdh/docker/Containerfile, preservingRHDH_MUST_GATHER_VERSIONand the Brew/KonfluxENV SUMMARY=/LABELfooter. Incrementsrelease="N"→N+1and thekonflux.additional-tagsversion-releasesuffix with a numeric boundary (2.0-1→2.0-2,2.0-10→2.0-11not2.0-20)..tekton/rhdh-must-gather-2-{pull,push}.yamlandmust-gather.prefetch_inputin.tekton-templates/components.yaml(genericvsgomod). Othercomponents.yamlentries are left untouched.sync/upstream_SHA_rhdh-must-gatherfrom a committed upstream HEAD. Refuses to pin a dirty HEAD (--allow-dirtystill does not write a SHA from uncommitted work).--skip-upstreamsyncs distgit + Tekton + SHA only.--skip-downstreambumps upstream only.Discovery and guards
--parent-dirfinds must-gather (1-must-gather, …) and midstream (4-rhdh,rhdh-downstream,rhidp-rhdh) only when the candidate hasdistgit/containers/rhdh-must-gather(skips a GitHubrhdhclone).hack/check-helm-binary-available.shon upstream; missingbash/rsyncis a hard error.git statusfailures (fatal: not a git repository, etc.) die instead of looking clean.bash,curl,git,rsync.Agent workflow (in
SKILL.md)--toagainst Helm releases.--check+--dry-run(preview)./mutation-gate.git diff+ references/verification.md (unit tests, Konflux, E2E triage including Helm 4 OCI stdout)./mutation-gate(Jira via/rhdh-jira-link).References: install-paths (CGW vs vendor, tarball layout), tekton-prefetch, verification, helm4-notes.
Related upstream work: rhdh-must-gather#284 (CGW binary), rhdh-must-gather#282 (vendored fallback).
Tests
tests/unit/test_must_gather_helm_bump.py(invoked viasys.executable):--tobump off the4.2.3fixture (lockfile/vendor stubs updateHELM_VERSION)vendor/helm, keeps websocat, scopes prefetch to must-gather, updates pull and push PLRs--dry-runwrites nothingrelease="10"/2.0-10→11/2.0-11git status, dirty-HEAD SHA refusal,rhdh-downstreamdiscovery, missing CGW probe script--skip-upstream --checkstill reportsmode=cgwwhen upstream CGW is available (stale distgitvendor/helmdoes not win)Test plan
uv run pytest tests/unit/test_must_gather_helm_bump.pyuv run pytest tests/unit/test_skill_catalog.py--check/--dry-runsmoke test against local must-gather + midstream checkoutsGenerated-by: cursor